Skip to content

✨ [Frontend] Feature: Saving pipeline #8054

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 15 commits into from
Jul 7, 2025
Merged

✨ [Frontend] Feature: Saving pipeline #8054

merged 15 commits into from
Jul 7, 2025

Conversation

@odeimaiz
Copy link
Member

@odeimaiz odeimaiz commented Jul 7, 2025 •
edited
Loading

What do these changes do?

This PR bring a small icon to the navigation bar that shows users when a study is being saved.

This makes sense only when users are dealing with pipelines, that's why it won't be enabled in standalone or app ui modes.

Reminder of how the save pipeline works:

  • the frontend keeps a version of the last saved model in the backend
  • if the model in the backend and the one in the frontend are not in sync, the frontend will try to patch the difference every 3" (non stop, if one fails it will retry)
  • if the model in the backend and the one in the frontend are not in sync, this new icon is shown

Saving

Related issue/s

How to test

Dev-ops

@odeimaiz odeimaiz added this to the Engage milestone Jul 7, 2025
@odeimaiz odeimaiz self-assigned this Jul 7, 2025
@odeimaiz odeimaiz added t:enhancement Improvement or request on an existing feature a:frontend issue affecting the front-end (area group) labels Jul 7, 2025
@odeimaiz odeimaiz requested a review from Copilot July 7, 2025 09:49
@odeimaiz odeimaiz marked this pull request as ready for review July 7, 2025 09:50
giancarloromeo
giancarloromeo approved these changes Jul 7, 2025
View reviewed changes
Copy link
Contributor

@giancarloromeo giancarloromeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

matusdrobuliak66
matusdrobuliak66 approved these changes Jul 7, 2025
View reviewed changes
Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement 👍 thanks

Copilot AI reviewed Jul 7, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a visual "Saving..." indicator to the navigation bar by tracking a new savePending flag on the Study model. It introduces a periodic diff check in the StudyEditor to update that flag and binds it to a new icon in the NavigationBar.

  • Adds a savePending property to Study and ignores it during serialization.
  • Introduces DIFF_CHECK_INTERVAL and a saving timer in StudyEditor to set savePending.
  • Binds study.changeSavePending events in NavigationBar to toggle a “Saving…” icon.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
services/static-webserver/client/source/class/osparc/navigation/NavigationBar.js Bind savePending to a new "saving-study-icon" control and register the icon in left-items
services/static-webserver/client/source/class/osparc/desktop/StudyEditor.js Define DIFF_CHECK_INTERVAL, start/stop a saving timer to call setSavePending, integrate into lifecycle
services/static-webserver/client/source/class/osparc/data/model/Study.js Declare savePending property with change event and exclude it from serialization
Comments suppressed due to low confidence (2)

services/static-webserver/client/source/class/osparc/desktop/StudyEditor.js:108

  • [nitpick] The name DIFF_CHECK_INTERVAL is generic—consider renaming it to something like SAVE_PENDING_CHECK_INTERVAL to clarify its specific role alongside AUTO_SAVE_INTERVAL.
    DIFF_CHECK_INTERVAL: 300,

services/static-webserver/client/source/class/osparc/desktop/StudyEditor.js:231

  • There are no tests covering the new saving indicator or the logic in __startSavingTimer. Adding unit or integration tests would help ensure this behavior works correctly and prevent regressions.
        this.__startSavingTimer();

GitHK
GitHK approved these changes Jul 7, 2025
View reviewed changes
Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice addition

sanderegg
sanderegg reviewed Jul 7, 2025
View reviewed changes
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nice! Maybe check the copilot review

@odeimaiz
Copy link
Member Author

odeimaiz commented Jul 7, 2025

@mergify queue

@mergify
Copy link
Contributor

mergify bot commented Jul 7, 2025 •
edited
Loading

queue

🟠 Waiting for conditions to match

  • -closed [📌 queue requirement]
  • any of: [🔀 queue conditions]
    • all of: [📌 queue conditions of queue default]
      • label=🤖-automerge
      • #approved-reviews-by >= 2 [🛡 GitHub branch protection]
      • #approved-reviews-by>=2
      • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
      • #changes-requested-reviews-by=0
      • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
      • #review-threads-unresolved=0
      • -conflict
      • -draft
      • base=master
      • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
      • label!=🤖-do-not-merge
      • any of: [🛡 GitHub branch protection]
        • check-skipped = deploy to dockerhub
        • check-neutral = deploy to dockerhub
        • check-success = deploy to dockerhub
      • any of: [🛡 GitHub branch protection]
        • check-success = system-tests
        • check-neutral = system-tests
        • check-skipped = system-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = unit-tests
        • check-neutral = unit-tests
        • check-skipped = unit-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = check OAS' are up to date
        • check-neutral = check OAS' are up to date
        • check-skipped = check OAS' are up to date
      • any of: [🛡 GitHub branch protection]
        • check-success = integration-tests
        • check-neutral = integration-tests
        • check-skipped = integration-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = build-test-images (frontend) / build-test-images
        • check-neutral = build-test-images (frontend) / build-test-images
        • check-skipped = build-test-images (frontend) / build-test-images
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of: [📌 queue -> configuration change requirements]
    • -mergify-configuration-changed
    • check-success = Configuration changed

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 7, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@odeimaiz odeimaiz merged commit 73cf774 into ITISFoundation:master Jul 7, 2025
59 checks passed
@odeimaiz odeimaiz deleted the feature/saving-study branch July 7, 2025 15:03
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Aug 5, 2025
Closed
88 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sanderegg sanderegg sanderegg left review comments

Copilot code review Copilot Copilot left review comments

@giancarloromeo giancarloromeo giancarloromeo approved these changes

@GitHK GitHK GitHK approved these changes

@matusdrobuliak66 matusdrobuliak66 matusdrobuliak66 approved these changes

@pcrespov pcrespov Awaiting requested review from pcrespov

@bisgaard-itis bisgaard-itis Awaiting requested review from bisgaard-itis

Assignees

@odeimaiz odeimaiz

Labels

a:frontend issue affecting the front-end (area group) t:enhancement Improvement or request on an existing feature

Projects

None yet

Milestone

Engage

Development

Successfully merging this pull request may close these issues.

5 participants

@odeimaiz @giancarloromeo @GitHK @sanderegg @matusdrobuliak66